Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BLADE-97] Refactor tx signer #115

Merged
merged 28 commits into from
Feb 26, 2024
Merged

Conversation

Stefan-Ethernal
Copy link

@Stefan-Ethernal Stefan-Ethernal commented Feb 14, 2024

Description

This PR reorganizes code for transactions signers in different signer structs and files for better separation of logic and responsibility.

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)

Checklist

  • I have assigned this PR to myself
  • I have added at least 1 reviewer
  • I have added the relevant labels
  • I have updated the official documentation
  • I have added sufficient documentation in code

Testing

  • I have tested this code with the official test suite
  • I have tested this code manually

@grujicf grujicf marked this pull request as ready for review February 14, 2024 09:53
Base automatically changed from feature/berlin to develop February 19, 2024 11:04
@Stefan-Ethernal Stefan-Ethernal changed the title [BLADE-97] Refactor tx signer (#113) [BLADE-97] Refactor tx signer Feb 19, 2024
@Stefan-Ethernal Stefan-Ethernal force-pushed the feat/refactor-tx-signers branch from 2ab4f84 to 5c97258 Compare February 19, 2024 12:38
helper/common/common.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
go.sum Outdated Show resolved Hide resolved
Copy link
Collaborator

@goran-ethernal goran-ethernal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not need stateTxSigner, but other signers must be aware that we have state tx as well. Those txs are in essence legacy tx, and their hash is calculated the same as LegacyTx. So in each signer which expects LegacyTx, we should also add StateTx as well as a type that can be processed by that signer.

crypto/txsigner.go Outdated Show resolved Hide resolved
crypto/txsigner_berlin.go Outdated Show resolved Hide resolved
crypto/txsigner_berlin.go Outdated Show resolved Hide resolved
crypto/txsigner_eip155.go Outdated Show resolved Hide resolved
crypto/txsigner_eip155.go Outdated Show resolved Hide resolved
crypto/txsigner_eip155.go Outdated Show resolved Hide resolved
crypto/txsigner_homestead.go Outdated Show resolved Hide resolved
crypto/txsigner_homestead.go Outdated Show resolved Hide resolved
crypto/txsigner_frontier.go Outdated Show resolved Hide resolved
crypto/txsigner_statetx.go Outdated Show resolved Hide resolved
@goran-ethernal goran-ethernal force-pushed the feat/refactor-tx-signers branch from f3a2686 to 749dbe7 Compare February 23, 2024 11:25
@goran-ethernal goran-ethernal self-requested a review February 23, 2024 12:45
@goran-ethernal goran-ethernal added the technical-debt Removal of technical debt label Feb 23, 2024
@goran-ethernal goran-ethernal force-pushed the feat/refactor-tx-signers branch from 3f85f20 to b7fda91 Compare February 23, 2024 14:04
@goran-ethernal goran-ethernal force-pushed the feat/refactor-tx-signers branch from e5aba84 to 397ef42 Compare February 25, 2024 18:25
goran-ethernal
goran-ethernal previously approved these changes Feb 26, 2024
@Stefan-Ethernal Stefan-Ethernal merged commit 256e511 into develop Feb 26, 2024
8 of 9 checks passed
@Stefan-Ethernal Stefan-Ethernal deleted the feat/refactor-tx-signers branch February 26, 2024 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical-debt Removal of technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants